-
-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip failing tests to allow 0.18.1 release on conda-forge #40
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
59b7da8
to
61e42c5
Compare
recipe/meta.yaml
Outdated
sha256: 6f871ab07657becb3cca158dc4af96a1a14c0a360add04664fba325da43537fa | ||
sha256: f861b988089b0ccc74c0be10a105ed2fe34a2a608d6f19e67147b4da26e39ac3 | ||
patches: | ||
- disable-0.18.1-failing-tests-on-osx.patch # [osx] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember correctly, ridge instability was also problematic on linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I had a few screw-ups sorry about that, I think it should be fine now but let's see what the CIs have to say about this.
61e42c5
to
ff188ee
Compare
Hmmm the commit message is slightly misleading because that was when I still thought that the tests were failing only on OSX. Please edit it if you use "squash and merge" ... Alternatively I could amend the commit but I would rather wait to make sure that the CI are green before doing so. |
Add ocefpaf and lesteve as recipe maintainers.
ff188ee
to
ed4ebb5
Compare
Eventhough I not fully agree on not patching a known resolved bug, I think this should go in. Maintainers ? |
We need another PR like this for branch |
Done in #41. |
Thanks. |
@jschueller I don't want to insist on this too much, but you are doing exactly that. There are many known fixed bugs in master, and trying to patch them here will probably lead to more bugs given the intricacies of the codebase and interdependencies. So you're patching one of them selectively. Alright. Why this one and not the rest? |
@amueller because this one messes with the unit tests that are run during the package build : if I were to patch I'd prefer to actually fixing the bug if possible instead of creating patching to skip it (and that's less work as the patch already exists). Again, I'm also fine with skipping them, that's just not how I usually do things. |
It's probably fine to path it, but it's still not as clear-cut. |
Can someone with the necessary rights please merge this one as well as #41? From the recipe, I am guessing that @amueller and @ogrisel have the necessary rights. If not @jakirkham maybe? |
FYI you should get rights to this repo in ~24hrs (or less) @lesteve. If you don't, please ping core. |
Thanks @amueller for merging this one!
Good to know, thanks @jakirkham. |
To put things in perspective w.r.t. releases and patching, I think the real issue that we are running into is not so much which kind of patches we use, but that we are now left to do this after the release has occurred. Maybe we should be doing some more |
Doing RCs with conda-forge would be really really good. |
Completely agree with this as I commented elsewhere and we will do our best to do this for the 0.19 release. |
@conda-forge/core for some reason it does not look like I have the rights on this repo although I was added to the recipe maintainers. Let me know if there is something I need to do. |
The team update script has been super flaky of late. Mainly a consequence of growing pains. Restarted it to see if we can get all the way through to When I have some more time, I'm planning on replacing it with a web service. That should scale better with our demands, provide better feedback to end users, run more quickly, and be less vulnerable. Was able to add the needed features to |
OK thanks a lot @jakirkham for the details! For completeness there is no hurry, I just happened to check today whether I had the rights. |
Thanks for understanding @lesteve. If it comes to it, I can always add you manually. Though I'm hoping we can fix the tooling first. |
From a scikit-learn developer this is what makes the most sense. It was already discussed in #39 and #24 but to sum up:
ping @jakirkham @jschueller @ogrisel @amueller.
fixes #38.
closes #39 closes #24.
I added @ocefpaf (since he added himself in #24) and myself to the recipe maintainers.